-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Managed fixes for async task tracking #123727
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds managed fixes for async task tracking by introducing tick count tracking for runtime async tasks and continuations to support debugging scenarios.
Changes:
- Adds two new ConcurrentDictionary fields to track task and continuation tick counts for debugging
- Implements methods to set, get, update, and remove tick count tracking for both tasks and continuations
- Integrates tick count tracking into the continuation allocation and dispatch flow
- Adds TPL event source tracing for synchronous work begin/end and operation begin/end events
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs | Adds static dictionaries and helper methods for tracking task and continuation tick counts, removes unnecessary blank line |
| src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs | Integrates tick count tracking into continuation lifecycle, adds ContinuationEqualityComparer, extends AsyncDispatcherInfo with Task field, adds TPL event source tracing |
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs
Show resolved
Hide resolved
| { | ||
| if (s_asyncDebuggingEnabled) | ||
| { | ||
| s_runtimeAsyncContinuationTicks ??= new Collections.Concurrent.ConcurrentDictionary<Continuation, long>(ContinuationEqualityComparer.Instance); |
Copilot
AI
Jan 29, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent namespace qualification: The ConcurrentDictionary type should be fully qualified as System.Collections.Concurrent.ConcurrentDictionary to match the field declarations on lines 184 and 187. The file does not have a using System.Collections.Concurrent; directive.
| { | ||
| if (s_asyncDebuggingEnabled) | ||
| { | ||
| s_runtimeAsyncContinuationTicks ??= new Collections.Concurrent.ConcurrentDictionary<Continuation, long>(ContinuationEqualityComparer.Instance); |
Copilot
AI
Jan 29, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent namespace qualification: The ConcurrentDictionary type should be fully qualified as System.Collections.Concurrent.ConcurrentDictionary to match the field declarations on lines 184 and 187. The file does not have a using System.Collections.Concurrent; directive.
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs
Show resolved
Hide resolved
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs
Show resolved
Hide resolved
| } | ||
| } | ||
|
|
||
| internal class ContinuationEqualityComparer : IEqualityComparer<Continuation> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What difference does it make specifying this comparer? Is this a workaround for missing functionality in the Continuation class where most objects have this default behavior but Continuations are special?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the virtual functions do not work on Continuations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can use a comment
| #else | ||
| [FieldOffset(8)] | ||
| #endif | ||
| public Task Task; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A comment describing which Task gets stored here and when would be helpful.
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs
Outdated
Show resolved
Hide resolved
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs
Outdated
Show resolved
Hide resolved
|
|
||
| ref byte resultLoc = ref nextContinuation != null ? ref nextContinuation.GetResultStorageOrNull() : ref GetResultStorage(); | ||
| long tickCount = Task.GetRuntimeAsyncContinuationTicks(curContinuation, out long tickCountVal) ? tickCountVal : Environment.TickCount64; | ||
| Task.UpdateRuntimeAsyncTaskTicks(this, tickCount); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would have guessed the tick count represented the first time a Task starts doing work but this seems to be updating the tick count every time a new continuation starts. Is that the desired behavior? It would be helpful to have a nice comment somewhere that describes what behavior we are trying to achieve with these side data structures.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added some comments on the methods and around them
| Task.UpdateRuntimeAsyncTaskTicks(this, tickCount); | ||
| Continuation? newContinuation = curContinuation.ResumeInfo->Resume(curContinuation, ref resultLoc); | ||
|
|
||
| Task.RemoveRuntimeAsyncContinuationTicks(curContinuation); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the continuation executes quickly is there a risk its already deleted before the debugger ever gets an opportunity to observe it? Would that matter or the debugger only cares about observing the tasks which are in-progress at the moment a debug event occurs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should add comments to all of this code. The UpdateRuntimeAsyncTaskTicks Is just to keep track of the start time of the invocation currently inflight, as it is not on the chain. Once its work is done we remove it.
| Continuation newContinuation = (Continuation)RuntimeTypeHandle.InternalAllocNoChecks(contMT); | ||
| #endif | ||
| prevContinuation.Next = newContinuation; | ||
| Task.SetRuntimeAsyncContinuationTicks(newContinuation, Environment.TickCount64); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we also mentioned swapping from TickCount to StopWatch if the debugger cares about getting higher precision timings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good because the etw events are also QPC timed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs
Outdated
Show resolved
Hide resolved
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs
Outdated
Show resolved
Hide resolved
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs
Outdated
Show resolved
Hide resolved
Added comments to clarify the purpose of the Task field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| // Needed because Continuations that are inflight have already been dequeued from the chain. | ||
| internal static System.Collections.Concurrent.ConcurrentDictionary<int, long>? s_runtimeAsyncTaskTicks; | ||
| // Dictionary to store debug info about runtime-async Continuations. | ||
| // The TickCount field stores the QPC tick count when the logical invocation to which the Continuation belongs started. | ||
| // The ID field stores a unique ID for the Continuation, similar to Task IDs. |
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment on line 181 says 'Dictionary that relates a runtime-async Task's ID to the QPC tick count' but doesn't explain the purpose or lifecycle of this mapping. Consider adding documentation explaining when entries are added/removed and how this differs from s_runtimeAsyncContinuationTicks.
| // Needed because Continuations that are inflight have already been dequeued from the chain. | |
| internal static System.Collections.Concurrent.ConcurrentDictionary<int, long>? s_runtimeAsyncTaskTicks; | |
| // Dictionary to store debug info about runtime-async Continuations. | |
| // The TickCount field stores the QPC tick count when the logical invocation to which the Continuation belongs started. | |
| // The ID field stores a unique ID for the Continuation, similar to Task IDs. | |
| // Entries are added when a runtime-async Task begins executing a logical invocation and removed when that invocation | |
| // completes (i.e. when the Task is no longer considered inflight). This mapping is used only for debugging/ | |
| // diagnostics (e.g. Async Causality and debugger support) to recover timing information for currently executing | |
| // runtime-async Tasks whose continuations have already been dequeued from the async method's continuation chain. | |
| internal static System.Collections.Concurrent.ConcurrentDictionary<int, long>? s_runtimeAsyncTaskTicks; | |
| // Dictionary to store debug info about runtime-async Continuations. | |
| // The key is the Continuation instance, and the value is RuntimeAsyncContinuationDebugInfo: | |
| // - The TickCount field stores the QPC tick count when the logical invocation to which the Continuation belongs | |
| // started (which may be earlier than the point at which the continuation itself begins executing). | |
| // - The ID field stores a unique ID for the Continuation, similar to Task IDs, to aid debugger correlation. | |
| // Entries are added when a runtime-async Continuation is created/scheduled and removed when that continuation has | |
| // finished executing or is otherwise no longer relevant. Unlike s_runtimeAsyncTaskTicks, which is keyed by Task ID | |
| // and tracks inflight Task invocations, this dictionary is keyed by Continuation and tracks per-continuation debug | |
| // information across the logical async invocation. |
| s_runtimeAsyncContinuationTicks ??= new Collections.Concurrent.ConcurrentDictionary<Continuation, RuntimeAsyncContinuationDebugInfo>(ContinuationEqualityComparer.Instance); | ||
| s_runtimeAsyncContinuationTicks.TryAdd(continuation, new RuntimeAsyncContinuationDebugInfo(tickCount)); |
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dictionary initialization pattern is duplicated across multiple methods (lines 225, 241, 254). Consider extracting this into a helper method or property to reduce duplication.
| RuntimeAsyncContinuationDebugInfo? debugInfo = null; | ||
| if (Task.s_asyncDebuggingEnabled) | ||
| { | ||
| debugInfo = Task.GetRuntimeAsyncContinuationDebugInfo(curContinuation, out RuntimeAsyncContinuationDebugInfo? debugInfoVal) ? debugInfoVal : new RuntimeAsyncContinuationDebugInfo(Stopwatch.GetTimestamp()); |
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is complex with nested ternary operations and method calls. Consider splitting into multiple lines for better readability, such as checking GetRuntimeAsyncContinuationDebugInfo first and then creating a new instance if needed.
| debugInfo = Task.GetRuntimeAsyncContinuationDebugInfo(curContinuation, out RuntimeAsyncContinuationDebugInfo? debugInfoVal) ? debugInfoVal : new RuntimeAsyncContinuationDebugInfo(Stopwatch.GetTimestamp()); | |
| RuntimeAsyncContinuationDebugInfo? debugInfoVal; | |
| if (Task.GetRuntimeAsyncContinuationDebugInfo(curContinuation, out debugInfoVal)) | |
| { | |
| debugInfo = debugInfoVal; | |
| } | |
| else | |
| { | |
| debugInfo = new RuntimeAsyncContinuationDebugInfo(Stopwatch.GetTimestamp()); | |
| } |
|
|
||
| if (newContinuation != null) | ||
| { | ||
| // we have a new Continuation that belongs to the same logical invocation as the previous; propagate debug info from previous continuation |
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment should clarify that 'invocation' refers to the logical async operation. Consider rewording to 'propagate debug info from the previous continuation to maintain the same logical invocation context'.
| // we have a new Continuation that belongs to the same logical invocation as the previous; propagate debug info from previous continuation | |
| // we have a new Continuation that belongs to the same logical async operation as the previous; propagate debug info from the previous continuation to maintain the same logical invocation context |
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs
Outdated
Show resolved
Hide resolved
| while (nc != null) | ||
| { | ||
| // On suspension we set tick info for all continuations that have not yet had it set. | ||
| Task.SetRuntimeAsyncContinuationTickCount(nc, Stopwatch.GetTimestamp()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Task.SetRuntimeAsyncContinuationTickCount(nc, Stopwatch.GetTimestamp()); | |
| Task.SetRuntimeAsyncContinuationTimestamp(nc, Stopwatch.GetTimestamp()); |
In .NET, TickCount typically means time stamp measured in milliseconds. I would avoid using that name for time stamps measured in some other units. Just call it Timestamp?
| { | ||
| while (nc != null) | ||
| { | ||
| // On suspension we set tick info for all continuations that have not yet had it set. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // On suspension we set tick info for all continuations that have not yet had it set. | |
| // On suspension we set timestamp for all continuations that have not yet had it set. |
This PR includes a few additions to the runtime-async handling loop to facilitate inspection by a debugger - specifically, Visual Studio.
Timestamp tracking for continuations. In VS there is a feature that lets you see, while F5 debugging, the relative start times and durations of different tasks. There is only one actual Task at the root of an await chain, but we intend to replicate the outward behavior in runtime-async by treating Continuations as "virtual Tasks". As such, we populate a dictionary to track the start times for each logical invocation.
TPL events. In Concord in asyncv1 we are notified of Task status changes via TPL events; specifically, we use these events to detect which Tasks are currently active and which thread each Task runs on. There are ways to do this without TPL in Concord; however, I believe a change to this would be across different (non-runtime async) Tasks and beyond the scope of this PR.